Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR provides a provides a proof-of-concept implementation for automatic unit-handling. (It hasn't been rigorously tested and the implementation could be a lot more efficient).
How to use Automatic Unit Handling
Currently, we have an opt-in system (if you don't opt-in, Grackle will use its existing behavior).
To specify that you want to use the automatic unit-handling you need to invoke the following1
When the user calls the
[local]_calculate_*
and[local]_solve_chemistry
families of functions, Grackle will automatically and internally calculate the current code_units based on the "current_a_value" (we'll talk about how that is specified in a moment). An error is reported if the user passes in something other than aNULL
pointer for thecode_units
argument of that in a moment.The current value of the cosmological scale factor is specified by the newly created
current_a_value
data-member of thegrackle_field_data
struct. If a user doesn't modify the default value, Grackle assumes that you want to use the value that thecode_units
struct held when it was passed intoinitialize_chemistry_data
orlocal_initialize_chemistry_data
.Additional ideas
Alternative mechanism for specifying
current_a_value
An alternative to the current mechanism for specifying the
current_a_value
is to store the value inside the chemistry_data_storage object. This would probably be achieved by introducing a function with the signatureMaking
code_units
unnecessaryCurrently, the user still needs to pass
code_units
intoinitialize_chemistry_data
orlocal_initialize_chemistry_data
, even when they are using the automatic unit-handling. We could make this unnecessary by introducing the following new parameters to thechemistry_data
struct.initial_a_value
initial_density_units
initial_time_units
initial_length_units
a_units
And we could encode the information currently held by
code_units::comoving_coordinates
within thechemistry_data::unit_handling
parameter.I actually think this would be a good thing (and would generally make it simpler to use grackle). I also think it would be easier to implement this as part of this PR (rather than as 2 separate steps). With that said, I think it's important to make it very clear that once the user calls
initialize_chemistry_data
orlocal_initialize_chemistry_data
, it would be disastrous to mutate any of these values.2Footnotes
by opting in, users are promising that they will use the gr_initialize_field_data function to initialize the
grackle_field_data
struct. If they don't do that problems could arise (and we don't currently have any good way to check whether they use that initialization function). ↩In general, I think we should always discourage people from mutating values after calling
initialize_chemistry_data
orlocal_initialize_chemistry_data
(unless they really know all of the internals of grackle). But in this case, we would need to take a particularly strong stance. ↩